Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ARM support for resolve-version #1321

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add ARM support for resolve-version #1321

wants to merge 4 commits into from

Conversation

mlarraz
Copy link

@mlarraz mlarraz commented Sep 10, 2024 •
edited
Loading

  • Copies the existing resolve-version-linux and resolve-version-darwin to resolve-version-linux-x64 and resolve-version-darwin-x64
  • Adds new Makefile steps to build these for ARM
  • Rewrite get_cpu() bash function to support ARM (and drop support for i686)
  • Use existing get_platform() function instead of get_os() to determine filename

I have not actually managed to build the new binaries myself as I'm not very experienced with Rust, but hoping somebody can help out.

I noticed there's a similar thing happening for yq but I didn't see any code handling the vendoring so I assume it was done manually. Presumably it will need ARM versions as well

This is meant to fix #1268

@mlarraz mlarraz requested a review from a team as a code owner September 10, 2024 20:00
@edmorley edmorley requested review from colincasey and removed request for a team September 11, 2024 08:14
@colincasey
Copy link
Contributor

@mlarraz thanks for the PR! I won't be able to fully review this until sometime next week due to other priorities but it looks like a good first step in addressing #1268.

As for the cross-compilation of the resolve-version binaries, that's a bit more complicated since the setup varies depending on the host/target but typically you need the following:

  • The standard library for the target installed with rustup target add {target} (e.g.; rustup target add aarch64-apple-darwin)
  • A linker for the platform (e.g.; brew install messense/macos-cross-toolchains/aarch64-unknown-linux-musl on Mac, sudo apt-get install g++-aarch64-linux-gnu libc6-dev-arm64-cross musl-tools on Linux)
  • It may also be necessary to set some environment variables so that cargo uses the correct toolchain (e.g.; CARGO_TARGET_AARCH64_UNKNOWN_LINUX_MUSL_LINKER and/or CC_AARCH64_UNKNOWN_LINUX_MUSL)

And you're right, the vendored yq binaries were manually sourced from their releases.

@mlarraz
Copy link
Author

mlarraz commented Sep 11, 2024

Thanks, I will work on adding the binaries.

I noticed a previous commit referenced compressing the yq binaries, but didn't go into detail. It doesn't look like it was using tar or some other container as the files are still directly executable.

@mlarraz mlarraz mentioned this pull request Sep 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colincasey colincasey Awaiting requested review from colincasey

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ARM64 when using Heroku-24 Docker images
2 participants
@mlarraz @colincasey